Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make BackendSampler set shots and stddev_upper_bound of returned QuasiDistribution #9312

Merged
merged 4 commits into from
Jan 18, 2023

Conversation

kevinsung
Copy link
Contributor

Summary

Fixes #9311

Details and comments

@kevinsung kevinsung requested review from a team, ikkoham and t-imamichi as code owners December 20, 2022 17:15
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

@kevinsung kevinsung changed the title Make BackendSampler pass shots to QuasiDistribution Make BackendSampler set shots and stddev_upper_bound of returned QuasiDistribution Dec 20, 2022
Copy link
Contributor

@Cryoris Cryoris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shots setting LGTM, but I'm less sure about the stddev_upper_bound since 1/N is only the asymptotic behavior, right? As the reference Sampler also doesn't set this I would be happy to merge without the stddev_upper_bound -- otherwise maybe someone more familiar with the QuasiDistribution class can have a look 🙂

@kevinsung
Copy link
Contributor Author

The logic for stddev_upper_bound is the same as at https://github.com/Qiskit/qiskit-ibm-runtime/blob/ce2e94028bfd60ce99c2fa1d5a9d56601e481e34/qiskit_ibm_runtime/utils/sampler_result_decoder.py#L32-L44

Maybe @t-imamichi or @nonhermitian can comment on the correctness. The stddev_upper_bound attribute is poorly documented, but my understanding is that it represents an upper bound on the standard deviation of the expected value of the operator ZZ...Z.

@woodsp-ibm woodsp-ibm added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: primitives Related to the Primitives module labels Dec 21, 2022
@Cryoris
Copy link
Contributor

Cryoris commented Dec 21, 2022

Ok, but if it were a bound on the stddev of the ZZ... operator then assume N=2 shots, measuring once +1 and once -1. Then you have a stddev of 1 but the upper bound would be 1/N = 1/2? That's why I'm a little confused about this argument...

@kevinsung
Copy link
Contributor Author

kevinsung commented Dec 21, 2022

Consider a quantum state |ψ⟩ for which the operator ZZ...Z is measured N times, yielding a collection of N numbers [+1, +1, -1, ...]. The confusion arises because there are two distinct quantities that may both be referred to as the "standard deviation":

  1. The standard deviation of the collection of N numbers [+1, +1, -1, ...].
  2. Let A be the average of the N numbers [+1, +1, -1, ...]. Then A can be considered to be drawn from a probability distribution P representing the result of an experiment in which |ψ⟩ is prepared and measured N times and the results are averaged (repeating this experiment would yield different values for A). The "standard deviation" could refer to the standard deviation of P.

In your comment above you refer to (1), but stddev_upper_bound refers to (2). Also, note that it's 1/sqrt(N), not 1/N.

@nonhermitian
Copy link
Contributor

Because the output can be a quasi-distribution with negative values one cannot use standard variance formulas. One can however get an upper-bound on this as described here: https://arxiv.org/abs/2006.14044,
valid only for operators |O| <= 1

@kevinsung
Copy link
Contributor Author

kevinsung commented Dec 21, 2022

Because the output can be a quasi-distribution with negative values one cannot use standard variance formulas. One can however get an upper-bound on this as described here: https://arxiv.org/abs/2006.14044, valid only for operators |O| <= 1

In this case it's a probability distribution though, since it's calculated as
https://github.com/Qiskit/qiskit-terra/blob/b271adb3087205f05cbc11288e582dfd20a48fa7/qiskit/primitives/backend_sampler.py#L169

@kevinsung kevinsung added this to the 0.23.0 milestone Jan 5, 2023
@mtreinish
Copy link
Member

In this case it's a probability distribution though, since it's calculated as

https://github.com/Qiskit/qiskit-terra/blob/b271adb3087205f05cbc11288e582dfd20a48fa7/qiskit/primitives/backend_sampler.py#L169

Yeah for the BackendSampler it's returning a normal probability distribution because there is no error mitigation being done as part of the generic implementation for any backend it does. It just computes the probability via division, we returned a QuasiDistribution because that's what the abstract sampler api dictates https://github.com/Qiskit/qiskit-terra/blob/c2215ae87b516e57357075d69753489f056cbabc/qiskit/primitives/base/sampler_result.py#L44

ikkoham
ikkoham previously approved these changes Jan 18, 2023
Copy link
Contributor

@ikkoham ikkoham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I forgot reviewing. LGTM. In this case, gamma is always 1, so this definition of the upper bound of standard deviation is correct.

Just one comment: this is not a bug, but enhancement.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3951860569

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.08%) to 84.87%

Files with Coverage Reduction New Missed Lines %
src/sabre_swap/mod.rs 2 99.54%
Totals Coverage Status
Change from base Build 3951859300: 0.08%
Covered Lines: 66044
Relevant Lines: 77818

💛 - Coveralls

@mergify mergify bot merged commit 72f0f52 into Qiskit:main Jan 18, 2023
@kevinsung kevinsung deleted the backend-sampler-shots branch January 18, 2023 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog mod: primitives Related to the Primitives module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QuasiDistribution returned by BackendSampler has incorrect shots
8 participants